HADOOP-19658. ABFS:Create and rename idempotency for FNS Blob#7914
HADOOP-19658. ABFS:Create and rename idempotency for FNS Blob#7914anujmodi2021 merged 8 commits intoapache:trunkfrom
Conversation
|
🎊 +1 overall
This message was automatically generated. |
|
🎊 +1 overall
This message was automatically generated. |
| AbfsRestOperation statusOp = null; | ||
| try { | ||
| // Check if the file already exists by calling GetPathStatus | ||
| statusOp = getPathStatus(path, false, tracingContext, null); |
There was a problem hiding this comment.
In case of override true, flow might come here with already a Head call done on path.
Can we avoid this head call in that case?
| assumeHnsDisabled(); | ||
| // Create a spy of AzureBlobFileSystemStore | ||
| AzureBlobFileSystemStore store = Mockito.spy(fs.getAbfsStore()); | ||
| assumeBlobServiceType(); |
There was a problem hiding this comment.
We can move all assume before any other statement
| // Create a spy of AzureBlobFileSystem | ||
| try (AzureBlobFileSystem fs = Mockito.spy( | ||
| (AzureBlobFileSystem) FileSystem.newInstance(getRawConfiguration()))) { | ||
| assumeHnsDisabled(); |
There was a problem hiding this comment.
Same here, move all assume to first few lines
|
🎊 +1 overall
This message was automatically generated. |
|
🎊 +1 overall
This message was automatically generated. |
anujmodi2021
left a comment
There was a problem hiding this comment.
+1
LGTM
Some nits added
| /**Flag to enable/disable sending client transactional ID during create/rename operations: {@value}*/ | ||
| public static final String FS_AZURE_ENABLE_CLIENT_TRANSACTION_ID = "fs.azure.enable.client.transaction.id"; | ||
| /**Flag to enable/disable create idempotency during create operation: {@value}*/ | ||
| public static final String FS_AZURE_ENABLE_CREATE_IDEMPOTENCY = "fs.azure.enable.create.idempotency"; |
There was a problem hiding this comment.
This is only for Blob Idempotency, may be we can keep config name accordingly
|
|
||
| public static final boolean DEFAULT_FS_AZURE_ENABLE_CLIENT_TRANSACTION_ID = true; | ||
|
|
||
| public static final boolean DEFAULT_FS_AZURE_ENABLE_ENABLE_CREATE_IDEMPOTENCY = true; |
There was a problem hiding this comment.
Typo, ENABLE added twice
| key = "HTTP Response"; | ||
| } | ||
| String values = StringUtils.join(";", entry.getValue()); | ||
| List<String> valuesList = entry.getValue(); |
There was a problem hiding this comment.
Due to null pointer exceptions on enabling AbfsIoUtils logging if value is null.
|
🎊 +1 overall
This message was automatically generated. |
| public boolean isConditionalCreateOverwriteEnabled() { | ||
| // If either the configured FS service type or the ingress service type is BLOB, | ||
| // conditional create-overwrite is not used. | ||
| if (getFsConfiguredServiceType() == AbfsServiceType.BLOB |
There was a problem hiding this comment.
why is this change needed?
|
🎊 +1 overall
This message was automatically generated. |
|
💔 -1 overall
This message was automatically generated. |
|
🎊 +1 overall
This message was automatically generated. |
============================================================
|
…#7914) Contributed by Anmol Asrani
) (#7929) Contributed by Anmol Asrani
This change introduces idempotency for create operation in the ABFS driver when using FNS-Blob. The update strengthens the client’s retry semantics to ensure that transient backend failures (e.g., HTTP 500) or ambiguous outcomes (where the client receives an error, but the operation has already succeeded on the service) are handled safely. This PR also includes test cases to validate the idempotency of rename operations.